Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle case where guard does not produce variable, but a . expr #295

Closed

Conversation

zachdaniel
Copy link
Contributor

@zachdaniel zachdaniel commented Aug 5, 2024

This is a simple fix but may not be the fully correct answer. This essentially discards information in the form of:

def fun(map) when is_atom(map.foo)

From what I can tell, it used to be the case that eventually a root node in guard AST would always resolve to a variable, but now it can resolve to var.value.

What I'm not sure of is whether or not this var.value can be used for something or if it should just be discarded as I'm discarding it.

@zachdaniel
Copy link
Contributor Author

I'm currently hunting down a case where ElixirLS is crashing while working on ash (so this should be reproducible by just running elixir-ls on the ash repo, no customizations needed).

I get the issue about a MatchError, and then this crash:

Error in process #PID<0.50.0> with exit value:
{{:badmatch, :error},
 [
   {:code_server, :reply_loading, 4, [file: ~c"code_server.erl", line: 1184]},
   {:code_server, :finish_on_load_2, 3, [file: ~c"code_server.erl", line: 1415]},
   {:code_server, :finish_on_load, 3, [file: ~c"code_server.erl", line: 1397]},
   {:code_server, :loop, 1, [file: ~c"code_server.erl", line: 183]}
 ]}
[logger: :removed_failing_filter, filter: {:logger_translator, {&Logger.Utils.translator/2, %{otp: true, sasl: false, translators: [{Logger.Translator, :translate}]}}}, owner: :primary, log_event: %{meta: %{error_logger: %{emulator: true, tag: :error}, pid: #PID<0.50.0>, time: 1722860178789716, gl: #PID<0.213.0>}, msg: {~c"Error in process ~p with exit value:~n~p~n", [#PID<0.50.0>, {{:badmatch, :error}, [{:code_server, :reply_loading, 4, [file: ~c"code_server.erl", line: 1184]}, {:code_server, :finish_on_load_2, 3, [file: ~c"code_server.erl", line: 1415]}, {:code_server, :finish_on_load, 3, [file: ~c"code_server.erl", line: 1397]}, {:code_server, :loop, 1, [file: ~c"code_server.erl", line: 183]}]}]}, level: :error}, reason: {:exit, {:DOWN, :code_server, {:get_object_code_for_loading, Logger.Translator}}, [{:code_server, :call, 1, [file: ~c"code_server.erl", line: 160]}, {:code, :ensure_loaded, 1, [file: ~c"code.erl", line: 560]}, {:error_handler, :undefined_function, 3, [file: ~c"error_handler.erl", line: 84]}, {Logger.Utils, :translate, 5, [file: ~c"lib/logger/utils.ex", line: 47]}, {Logger.Utils, :translator, 2, [file: ~c"lib/logger/utils.ex", line: 28]}]}]
supervisor: {local,'Elixir.ElixirLS.LanguageServer.Supervisor'}
    errorContext: child_terminated
    reason: shutdown
    offender: [{pid,<0.224.0>},
               {id,'Elixir.ElixirLS.LanguageServer.Tracer'},
               {mfargs,
                   {'Elixir.ElixirLS.LanguageServer.Tracer',start_link,[[]]}},
               {restart_type,permanent},
               {significant,false},
               {shutdown,5000},
               {child_type,worker}]
supervisor: {local,'Elixir.ElixirLS.LanguageServer.Supervisor'}
    errorContext: shutdown
    reason: reached_max_restart_intensity
    offender: [{pid,<0.224.0>},
               {id,'Elixir.ElixirLS.LanguageServer.Tracer'},
               {mfargs,
                   {'Elixir.ElixirLS.LanguageServer.Tracer',start_link,[[]]}},
               {restart_type,permanent},
               {significant,false},
               {shutdown,5000},
               {child_type,worker}]
** (exit) exited in: GenServer.call(ElixirLS.LanguageServer.JsonRpc, {:packet, %{"jsonrpc" => "2.0", "method" => "textDocument/didChange", "params" => %{"contentChanges" => [%{"range" => %{"end" => %{"character" => 25, "line" => 22}, "start" => %{"character" => 25, "line" => 22}}, "text" => "y"}], "textDocument" => %{"uri" => "file:///Users/zachdaniel/dev/ash/ash/lib/ash/actions/aggregate.ex", "version" => 17}}}}, :infinity)
    ** (EXIT) no process: the process is not alive or there's no process currently associated with the given name, possibly because its application isn't started
    (elixir 1.17.2) lib/gen_server.ex:1121: GenServer.call/3
    (elixir 1.17.2) lib/stream.ex:482: anonymous fn/4 in Stream.each/2
    (elixir 1.17.2) lib/stream.ex:1717: Stream.do_element_resource/6
    (elixir 1.17.2) lib/stream.ex:1891: Enumerable.Stream.do_each/4
    (elixir 1.17.2) lib/stream.ex:690: Stream.run/1
    /opt/homebrew/Cellar/elixir-ls/0.22.1/libexec/launch.exs:18: (file)
    (elixir 1.17.2) lib/code.ex:1491: Code.require_file/2

There is one other error I'm going to start looking into next as well:

Unable to format docstring expression:

{:<<>>, [delimiter: "\"\"\"", indentation: 2, line: 4139, column: 8],
 [
   "Manages the related records by creating, updating, or destroying them as necessary.\n\nKeep in mind that the default values for all `on_*` are `:ignore`, meaning nothing will happen\nunless you provide instructions.\n\nThe input provided to `manage_relationship` should be a map, in the case of to_one relationships, or a list of maps\nin the case of to_many relationships. The following steps are followed for each input provided:\n\n- The input is checked against the currently related records to find any matches. The primary key and unique identities are used to find matches.\n- For any input that had a match in the current relationship, the `:on_match` behavior is triggered\n- For any input that does not have a match:\n  - if there is `on_lookup` behavior:\n    - we try to find the record in the data layer.\n    - if the record is found, the on_lookup behavior is triggered\n    - if the record is not found, the `on_no_match` behavior is triggered\n  - if there is no `on_lookup` behavior:\n    - the `on_no_match` behavior is triggered\n- finally, for any records present in the *current relationship* that had no match *in the input*, the `on_missing` behavior is triggered\n\n## Options\n\n",
   {:"::", [line: 4161, column: 3],
    [
      {{:., [line: 4161, column: 3], [Kernel, :to_string]},
       [
         from_interpolation: true,
         closing: [line: 4161, column: 37],
         line: 4161,
         column: 3
       ],
       [
         {{:., [line: 4161, column: 18],
           [
             {:__aliases__,
              [last: [line: 4161, column: 11], line: 4161, column: 5],
              [:Spark, :Options]},
             :docs
           ]}, [closing: [line: 4161, column: 36], line: 4161, column: 19],
          [
            {:@, [line: 4161, column: 24],
             [{:manage_opts, [line: 4161, column: 25], nil}]}
          ]}
       ]},
      {:binary, [line: 4161, column: 3], nil}
    ]},
   "\n\nEach call to this function adds new records that will be handled according to their options. For example,\nif you tracked \"tags to add\" and \"tags to remove\" in separate fields, you could input them like so:\n\n```elixir\nchangeset\n|> Ash.Changeset.manage_relationship(\n  :tags,\n  [%{name: \"backend\"}],\n  on_lookup: :relate, #relate that tag if it exists in the database\n  on_no_match: :error # error if a tag with that name doesn't exist\n)\n|> Ash.Changeset.manage_relationship(\n  :tags,\n  [%{name: \"frontend\"}],\n  on_no_match: :error, # error if a tag with that name doesn't exist in the relationship\n  on_match: :unrelate # if a tag with that name is related, unrelate it\n)\n```\n\nWhen calling this multiple times with the `on_missing` option set, the list of records that are considered missing are checked\nafter each set of inputs is processed. For example, if you manage the relationship once with `on_missing: :unrelate`, the records\nmissing from your input will be removed, and *then* your next call to `manage_relationship` will be resolved (with those records unrelated).\nFor this reason, it is suggested that you don't call this function multiple times with an `on_missing` instruction, as you may be\nsurprised by the result.\n\nIf you want the input to update existing entities, you need to ensure that the primary key (or unique identity) is provided as\npart of the input. See the example below:\n\n    changeset\n    |> Ash.Changeset.manage_relationship(\n      :comments,\n      [%{rating: 10, contents: \"foo\"}],\n      on_no_match: {:create, :create_action},\n      on_missing: :ignore\n    )\n    |> Ash.Changeset.manage_relationship(\n      :comments,\n      [%{id: 10, rating: 10, contents: \"foo\"}],\n      on_match: {:update, :update_action},\n      on_no_match: {:create, :create_action})\n\nThis is a simple way to manage a relationship. If you need custom behavior, you can customize the action that is\ncalled, which allows you to add arguments/changes. However, at some point you may want to forego this function\nand make the changes yourself. For example:\n\n    input = [%{id: 10, rating: 10, contents: \"foo\"}]\n\n    changeset\n    |> Ash.Changeset.after_action(fn _changeset, result ->\n      # An example of updating comments based on a result of other changes\n      for comment <- input do\n        comment = Ash.get(Comment, comment.id)\n\n        comment\n        |> Map.update(:rating, 0, &(&1 * result.rating_weight))\n        |> Ash.update!()\n      end\n\n      {:ok, result}\n    end)\n\n## Using records as input\n\nRecords can be supplied as the input values. If you do:\n\n* if it would be looked up due to `on_lookup`, the record is used as-is\n* if it would be created due to `on_no_match`, the record is used as-is\n* Instead of specifying `join_keys`, those keys must go in `__metadata__.join_keys`. If `join_keys` is specified in the options, it is ignored.\n\nFor example:\n\n```elixir\npost1 =\n  changeset\n  |> Ash.create!()\n  |> Ash.Resource.put_metadata(:join_keys, %{type: \"a\"})\n\npost1 =\n  changeset2\n  |> Ash.create!()\n  |> Ash.Resource.put_metadata(:join_keys, %{type: \"b\"})\n\nauthor = Ash.create!(author_changeset)\n\nAsh.Changeset.manage_relationship(\n  author,\n  :posts,\n  [post1, post2],\n  on_lookup: :relate\n)\n```\n"
 ]}

** (ArgumentError) cannot invoke @/1 outside module
    (elixir 1.17.2) lib/kernel.ex:6811: Kernel.assert_module_scope/3
    (elixir 1.17.2) expanding macro: Kernel.@/1
    nofile:4161: (file)
    (elixir 1.17.2) expanding macro: Kernel.to_string/1
    nofile:4161: (file)

@lukaszsamson
Copy link
Collaborator

lukaszsamson commented Aug 5, 2024

What I'm not sure of is whether or not this var.value can be used for something or if it should just be discarded as I'm discarding it.

We could use it to infer the type of map to be a map with keys including foo. That will be used for completions

I get the issue about a MatchError, and then this crash:

I can see at least 2 things happening here. First, the code_server is crashing. This could be an OTP issue. Then Logger.Utils.translator fails to translate the error message and gets removed by logger. This looks like elixir logger issue. Finally, the tracer crashes reaching max restart intensity. It's hard to tell if this is connected to the previous crashes.

There is one other error I'm going to start looking into next as well:

this one is rather harmless. The AST seems to contain @doc with dynamic content and the engine is unable to extract a string from it.

@zachdaniel
Copy link
Contributor Author

Yeah, the doctoring warning seems harmless.

We could use it to infer the type of map to be a map with keys including foo. That will be used for completions

What format would I return to indicate that here?

I can see at least 2 things happening here. First, the code_server is crashing. This could be an OTP issue. Then Logger.Utils.translator fails to translate the error message and gets removed by logger. This looks like elixir logger issue. Finally, the tracer crashes reaching max restart intensity. It's hard to tell if this is connected to the previous crashes.

Does this sound like something that could/should ultimately crash the language server? Those messages may not be the true cause, but are currently the only thing I have to go on.

this one is rather harmless. The AST seems to contain @doc with dynamic content and the engine is unable to extract a string from it.

Yeah, looked into it, makes sense 👍

@josevalim
Copy link

This is an Erlang bug: erlang/otp#8510

Once the code server crashes, it is game over. Elixir, LS, and others will misbehave. I will try to prioritize a fix for #8510.

@lukaszsamson
Copy link
Collaborator

I implemented type inference for map field access in guards in b117cae (part of #293). Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants